Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move payload_manager.notify_commit to after commit #15361

Merged
merged 1 commit into from
Nov 22, 2024
Merged

Move payload_manager.notify_commit to after commit #15361

merged 1 commit into from
Nov 22, 2024

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented Nov 21, 2024 •
edited
Loading

Description

This prevents QS from GCing batches before they are actually committed.

How Has This Been Tested?

Type of Change

  • Bug fix

Which Components or Systems Does This Change Impact?

  • Validator Node

@trunk-io Trunk.io
Copy link

trunk-io bot commented Nov 21, 2024 •
edited
Loading

⏱️ 1h 52m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
check 13m 🟩 🟩 🟩 
execution-performance / test-target-determinator 13m 🟩 🟩 🟩 
rust-move-tests 13m 🟩 
test-target-determinator 13m 🟩 🟩 🟩 
rust-move-tests 12m 🟩 
rust-move-tests 12m 🟩 
rust-cargo-deny 5m 🟩 🟩 🟩 
fetch-last-released-docker-image-tag 5m 🟩 🟩 🟩 
rust-doc-tests 5m 🟩 
rust-doc-tests 5m 🟩 
rust-doc-tests 5m 🟩 
check-dynamic-deps 5m 🟩 🟩 🟩 🟩 
semgrep/ci 2m 🟩 🟩 🟩 🟩 
general-lints 2m 🟩 🟩 🟩 
file_change_determinator 40s 🟩 🟩 🟩 

🚨 1 job on the last run was significantly faster/slower than expected

Job Duration vs 7d avg Delta
execution-performance / single-node-performance 10s 17m -99%

settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io

@msmouse msmouse added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Nov 21, 2024
This was referenced Nov 21, 2024
Merged
Merged
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@msmouse msmouse marked this pull request as ready for review November 22, 2024 02:09
@msmouse msmouse enabled auto-merge (squash) November 22, 2024 17:11
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@msmouse msmouse merged commit 7d41b65 into main Nov 22, 2024
134 of 144 checks passed
@msmouse msmouse deleted the 1121-alden-commit-hook branch November 22, 2024 17:43
@sherry-x sherry-x added the v1.23 label Nov 22, 2024
github-actions bot pushed a commit that referenced this pull request Nov 22, 2024
@msmouse
Move payload_manager.notify_commit to after commit (#15361)
95d0d58 
(cherry picked from commit 7d41b65)

# Conflicts:
#	consensus/src/pipeline/pipeline_builder.rs
#	consensus/src/state_computer.rs
@github-actions github-actions bot mentioned this pull request Nov 22, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ aptos-release-v1.23

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on ed5a60f6941d5d58195c8b5e96cf162b32449f81

two traffics test: inner traffic : committed: 14444.90 txn/s, latency: 2749.08 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 4700 ms), latency samples: 5492280
two traffics test : committed: 100.06 txn/s, latency: 1608.20 ms, (p50: 1400 ms, p70: 1500, p90: 1600 ms, p99: 8100 ms), latency samples: 1800
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.986, avg: 1.498", "ConsensusProposalToOrdered: max: 0.319, avg: 0.291", "ConsensusOrderedToCommit: max: 0.387, avg: 0.374", "ConsensusProposalToCommit: max: 0.679, avg: 0.665"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.07s no progress at version 43212 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.59s no progress at version 2835633 (avg 8.59s) [limit 15].
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite compat success on 327445f70a3b9474f13a792e8ada9b85809dec86 ==> ed5a60f6941d5d58195c8b5e96cf162b32449f81

Compatibility test results for 327445f70a3b9474f13a792e8ada9b85809dec86 ==> ed5a60f6941d5d58195c8b5e96cf162b32449f81 (PR)
1. Check liveness of validators at old version: 327445f70a3b9474f13a792e8ada9b85809dec86
compatibility::simple-validator-upgrade::liveness-check : committed: 14662.96 txn/s, latency: 2325.99 ms, (p50: 2100 ms, p70: 2200, p90: 2500 ms, p99: 6100 ms), latency samples: 471140
2. Upgrading first Validator to new version: ed5a60f6941d5d58195c8b5e96cf162b32449f81
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7819.98 txn/s, latency: 3547.73 ms, (p50: 3600 ms, p70: 4300, p90: 4600 ms, p99: 4900 ms), latency samples: 145220
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7638.29 txn/s, latency: 4151.92 ms, (p50: 4200 ms, p70: 4300, p90: 6400 ms, p99: 6700 ms), latency samples: 253240
3. Upgrading rest of first batch to new version: ed5a60f6941d5d58195c8b5e96cf162b32449f81
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6951.37 txn/s, latency: 4042.40 ms, (p50: 4500 ms, p70: 4800, p90: 5300 ms, p99: 5500 ms), latency samples: 128380
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7135.93 txn/s, latency: 4510.84 ms, (p50: 4800 ms, p70: 5000, p90: 6100 ms, p99: 6600 ms), latency samples: 238840
4. upgrading second batch to new version: ed5a60f6941d5d58195c8b5e96cf162b32449f81
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 11361.65 txn/s, latency: 2438.12 ms, (p50: 2500 ms, p70: 2700, p90: 3300 ms, p99: 3600 ms), latency samples: 195660
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10059.22 txn/s, latency: 3096.46 ms, (p50: 2600 ms, p70: 3200, p90: 5800 ms, p99: 7000 ms), latency samples: 325120
5. check swarm health
Compatibility test for 327445f70a3b9474f13a792e8ada9b85809dec86 ==> ed5a60f6941d5d58195c8b5e96cf162b32449f81 passed
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on 327445f70a3b9474f13a792e8ada9b85809dec86 ==> ed5a60f6941d5d58195c8b5e96cf162b32449f81

Compatibility test results for 327445f70a3b9474f13a792e8ada9b85809dec86 ==> ed5a60f6941d5d58195c8b5e96cf162b32449f81 (PR)
Upgrade the nodes to version: ed5a60f6941d5d58195c8b5e96cf162b32449f81
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1414.70 txn/s, submitted: 1417.91 txn/s, failed submission: 3.21 txn/s, expired: 3.21 txn/s, latency: 2185.28 ms, (p50: 1800 ms, p70: 2400, p90: 3600 ms, p99: 5100 ms), latency samples: 123560
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1326.12 txn/s, submitted: 1329.67 txn/s, failed submission: 3.55 txn/s, expired: 3.55 txn/s, latency: 2242.56 ms, (p50: 2100 ms, p70: 2400, p90: 3200 ms, p99: 5000 ms), latency samples: 119660
5. check swarm health
Compatibility test for 327445f70a3b9474f13a792e8ada9b85809dec86 ==> ed5a60f6941d5d58195c8b5e96cf162b32449f81 passed
Upgrade the remaining nodes to version: ed5a60f6941d5d58195c8b5e96cf162b32449f81
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1376.45 txn/s, submitted: 1379.88 txn/s, failed submission: 3.43 txn/s, expired: 3.43 txn/s, latency: 2278.43 ms, (p50: 2100 ms, p70: 2400, p90: 3300 ms, p99: 5100 ms), latency samples: 120320
Test Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zekun000 zekun000 zekun000 approved these changes

@ibalajiarun ibalajiarun ibalajiarun approved these changes

@sasha8 sasha8 Awaiting requested review from sasha8 sasha8 is a code owner

Assignees
No one assigned
Labels
CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR v1.23
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@msmouse @ibalajiarun @zekun000 @sherry-x